-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat: add eol page #7990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add eol page #7990
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ran with |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7990 +/- ##
==========================================
+ Coverage 72.82% 73.21% +0.38%
==========================================
Files 96 97 +1
Lines 8328 8452 +124
Branches 214 227 +13
==========================================
+ Hits 6065 6188 +123
- Misses 2262 2263 +1
Partials 1 1 ☔ View full report in Codecov by Sentry. |
I feel this page should use a layout without the sidebar/metabar FYI |
@avivkeller can we use the new website for CVEs? https://www.cve.org/CVERecord?id=CVE-2025-23166 instead of cve.mitre.org? |
Also this description: "There are 4+ known vulnerabilities associated with this Node.js release. Please review their severity and details to understand the potential impact." not sure if it is fitting. I think we should lean more into what CVEs are how to understand them what having these issues are... IDK; Throwing the "Please review their severity and details to understand the potential impact." to the end-user might not be ideal. I mean in the end that's whaty they're going to do anyways, but we could use such section to explain actual concrete details of what CVEs means, what clicking these links means, idk. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: Claudio Wunder <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bmuenzenmeyer I left some final comments here. But we're almost there!
Co-authored-by: Claudio Wunder <[email protected]> Signed-off-by: Brian Muenzenmeyer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
'4', | ||
'5', | ||
]); | ||
it('treats <= as inclusive of the specified major only (based on current implementation)', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's unfortunate to see this business logic and testing paved over without discussion.
This was implemented directly from the blog post assertion in bold:
As the security advisory notes, "End-of-Life versions are always affected when a security release occurs", meaning Node.js 18 and all earlier versions have these same vulnerabilities but will never receive patches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I didn't make changes to the logic, should still be the same constraint as it was before? Feel free to open a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries. the test suite will be able to illustrate what we think is right. i will open an issue when i have time and seek clarity from matteo and security
import { SEVERITY_ORDER } from '#site/next.constants.mjs'; | ||
import type { Severity, Vulnerability } from '#site/types'; | ||
|
||
import VulnerabilityChip from './VulnerabilityChip'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the refactor created some relative imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think same level relative imports are OK, no? cc @nodejs/nodejs-website
Description
We now have a dedicated EOL page!
Changes not directly related to the scope
Validation
EOL Page - live preview at https://nodejs-org-git-eol-openjs.vercel.app/en/eol
EOL / Vulnerability Table
Details
Link updated here
and here
Vulnerability Blog Posts
Related Issues
closes #7906
closes #7899
Check List
All other links on all alert boxes across the website (blog post from Matteo, Download pages, Version modal, etc) go to the /EOL page
- did not do Matteo's blog post yet - would rather someone else choose to editorialize that.pnpm format
to ensure the code follows the style guide.pnpm test
to check if all tests are passing.pnpm build
to check if the website builds without errors.